-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Tweak ofga list objects return type #1032
Tweak ofga list objects return type #1032
Conversation
internal/openfga/openfga.go
Outdated
@@ -111,7 +107,7 @@ func (o *OFGAClient) RemoveRelation(ctx context.Context, tuples ...Tuple) error | |||
} | |||
|
|||
// ListObjects returns all object IDs of <objType> that a user has the relation <relation> to. | |||
func (o *OFGAClient) ListObjects(ctx context.Context, user string, relation string, objType string, contextualTuples []Tuple) ([]string, error) { | |||
func (o *OFGAClient) ListObjects(ctx context.Context, user string, relation string, objType string, contextualTuples []Tuple) ([]cofga.Entity, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's not directly use cofga
types in the internal/openfga
package API. There's Tag
(which is an alias for cofga.Entity
) in the package that you can use.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah good point, thank you
internal/openfga/openfga.go
Outdated
@@ -80,7 +80,7 @@ func (o *OFGAClient) getRelatedObjects(ctx context.Context, tuple Tuple, pageSiz | |||
// must NOT include the ID, i.e., | |||
// | |||
// - "group:" vs "group:mygroup", where "mygroup" is the ID and the correct objType would be "group". | |||
func (o *OFGAClient) listObjects(ctx context.Context, user string, relation string, objType string, contextualTuples []Tuple) (objectIds []string, err error) { | |||
func (o *OFGAClient) listObjects(ctx context.Context, user string, relation string, objType string, contextualTuples []Tuple) (objectIds []cofga.Entity, err error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See the next comment.
internal/openfga/openfga_test.go
Outdated
@@ -435,9 +435,12 @@ func (s *openFGATestSuite) TestListObjectsWithContextualTuples(c *gc.C) { | |||
"30000000-0000-0000-0000-000000000000", | |||
} | |||
|
|||
expected := make([]string, len(modelUUIDs)) | |||
expected := make([]cofga.Entity, len(modelUUIDs)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The same thing.
internal/openfga/openfga_test.go
Outdated
for i, v := range modelUUIDs { | ||
expected[i] = "model:" + v | ||
expected[i] = cofga.Entity{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here, too.
internal/openfga/openfga_test.go
Outdated
@@ -474,8 +477,8 @@ func (s *openFGATestSuite) TestListObjectsWithContextualTuples(c *gc.C) { | |||
c.Assert(cmp.Equal( | |||
ids, | |||
expected, | |||
cmpopts.SortSlices(func(want string, expected string) bool { | |||
return want < expected | |||
cmpopts.SortSlices(func(want cofga.Entity, expected cofga.Entity) bool { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here, too.
internal/openfga/openfga_test.go
Outdated
@@ -489,9 +492,12 @@ func (s *openFGATestSuite) TestListObjectsWithPeristedTuples(c *gc.C) { | |||
"30000000-0000-0000-0000-000000000000", | |||
} | |||
|
|||
expected := make([]string, len(modelUUIDs)) | |||
expected := make([]cofga.Entity, len(modelUUIDs)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here, too.
internal/openfga/openfga_test.go
Outdated
for i, v := range modelUUIDs { | ||
expected[i] = "model:" + v | ||
expected[i] = cofga.Entity{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here, too.
internal/openfga/openfga_test.go
Outdated
@@ -531,8 +537,8 @@ func (s *openFGATestSuite) TestListObjectsWithPeristedTuples(c *gc.C) { | |||
c.Assert(cmp.Equal( | |||
ids, | |||
expected, | |||
cmpopts.SortSlices(func(want string, expected string) bool { | |||
return want < expected | |||
cmpopts.SortSlices(func(want cofga.Entity, expected cofga.Entity) bool { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here, too.
@@ -217,7 +217,15 @@ func (u *User) UnsetApplicationOfferAccess(ctx context.Context, resource names.A | |||
|
|||
// ListModels returns a slice of model UUIDs this user has at least reader access to. | |||
func (u *User) ListModels(ctx context.Context) ([]string, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I checked this method is also called in internal/jujuapi/jimm.go:500 again with splitting around :
. Could you please remove that part, too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@babakks That should already be removed, are you still seeing it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right, sorry.
Description
Follow up to #1030, instead of parsing ofga entities which are of the form kind:id, instead let's return the entity further up the call stack and then allow the caller to extract the bits they need.
Engineering checklist
Check only items that apply